Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3882] Add a new tree event to support non explorer tree descriptions #3884

Merged
merged 1 commit into from
Sep 24, 2024
Merged

[3882] Add a new tree event to support non explorer tree descriptions #3884

merged 1 commit into from
Sep 24, 2024

Conversation

jerome-obeo
Copy link
Contributor

Pull request template

General purpose

What is the main goal of this pull request?

  • Bug fixes
  • New features
  • Documentation
  • Cleanup
  • Tests
  • Build / releng

Project management

  • Has the pull request been added to the relevant project and milestone? (Only if you know that your work is part of a specific iteration such as the current one)
  • Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
  • Have the relevant issues been added to the pull request?
  • Have the relevant labels been added to the issues? (area:, difficulty:, type:)
  • Have the relevant issues been added to the same project and milestone as the pull request?
  • Has the CHANGELOG.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc? (Including changes in the GraphQL API)
  • In case of a change with a visual impact, are there any screenshots in the CHANGELOG.adoc? For example in doc/screenshots/2022.5.0-my-new-feature.png

Architectural decision records (ADR)

  • Does the title of the commit contributing the ADR start with [doc]?
  • Are the ADRs mentioned in the relevant section of the CHANGELOG.adoc?

Dependencies

  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
  • Are the new dependencies justified in the CHANGELOG.adoc?

Frontend

This section is not relevant if your contribution does not come with changes to the frontend.

General purpose

  • Is the code properly tested? (Plain old JavaScript tests for business code and tests based on React Testing Library for the components)

Typing

We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).

  • Variables have a proper type
  • Functions’ arguments have a proper type
  • Functions’ return type are specified
  • Hooks are properly typed:
    • useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
    • useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
    • useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
    • useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
    • useState<STATE_TYPE>(…)
  • All components have a proper typing for their props
  • No useless optional chaining with ?. (if the GraphQL API specifies that a field cannot be null, do not treat it has potentially null for example)
  • Nullable values have a proper type (for example let diagram: Diagram | null = null;)

Backend

This section is not relevant if your contribution does not come with changes to the backend.

General purpose

  • Are all the event handlers tested?
  • Are the event processor tested?
  • Is the business code (services) tested?
  • Are diagram layout changes tested?

Architecture

  • Are data structure classes properly separated from behavioral classes?
  • Are all the relevant fields final?
  • Is any data structure mutable? If so, please write a comment indicating why
  • Are behavioral classes either stateless or side effect free?

Review

How to test this PR?

Please describe here the various use cases to test this pull request

  • Has the Kiwi TCMS test suite been updated with tests for this contribution?

@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch from b3da927 to 122007e Compare August 26, 2024 18:55
@frouene frouene added this to the 2024.9.0 milestone Aug 27, 2024
@frouene frouene linked an issue Aug 27, 2024 that may be closed by this pull request
Add a new tree event to handle tree description which are not explorer-related #3882
Closed
1 task
@jerome-obeo jerome-obeo mentioned this pull request Aug 27, 2024
Merged
39 tasks
@jerome-obeo jerome-obeo changed the title Add a new tree event to handle tree description which are not explorer-related [3882] Add a new tree event to support non explorer tree descriptions Aug 28, 2024
@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch 7 times, most recently from 63c89f0 to 5a2801d Compare August 30, 2024 09:13
@frouene frouene self-requested a review September 3, 2024 14:55
@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch from 5a2801d to e567676 Compare September 4, 2024 09:17
@jerome-obeo jerome-obeo requested a review from frouene September 4, 2024 09:24
@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch 5 times, most recently from 0074943 to 94e74b7 Compare September 13, 2024 08:47
@frouene frouene force-pushed the jgo/enh/add_tree_event branch from 94e74b7 to 445b3a5 Compare September 16, 2024 09:36
sbegaudeau
sbegaudeau previously requested changes Sep 16, 2024
View reviewed changes
Copy link
Member

@sbegaudeau sbegaudeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll just block the merge of this feature for now since there are two PRs that are a bit related to this that I need to see merged first:

  • [3763] Implements the Selection Dialog TreeView
  • [3976] Remove IRepresentationConfiguration
  • And then this one

Since both PR already had to experience some major impact due to the integration of the previous PR on this topic, I have to make sure they get merged now that they just finish rebasing their work.

@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch from 445b3a5 to eb0c2cc Compare September 23, 2024 07:00
@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch from eb0c2cc to 1b8980c Compare September 23, 2024 09:58
@jerome-obeo jerome-obeo marked this pull request as draft September 23, 2024 10:00
@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch 2 times, most recently from 2a212fa to 8980df7 Compare September 23, 2024 15:10
@jerome-obeo jerome-obeo marked this pull request as ready for review September 23, 2024 15:10
@jerome-obeo jerome-obeo force-pushed the jgo/enh/add_tree_event branch from 8980df7 to b6ecb9c Compare September 23, 2024 15:39
@frouene
Copy link
Contributor

frouene commented Sep 24, 2024 •
edited
Loading

I'll just block the merge of this feature for now since there are two PRs that are a bit related to this that I need to see merged first:

* [3763] Implements the Selection Dialog TreeView

* [3976] Remove IRepresentationConfiguration

* And then this one

Since both PR already had to experience some major impact due to the integration of the previous PR on this topic, I have to make sure they get merged now that they just finish rebasing their work.

Could be merge now that the other pending PRs have been integrated

@jerome-obeo @frouene
[3882] Add a new tree event to support non explorer tree descriptions
779f77e 
+ an example of tree representation for Domain elements

Bug: eclipse-sirius#3882
Signed-off-by: Jerome Gout <[email protected]>
@frouene frouene force-pushed the jgo/enh/add_tree_event branch from b6ecb9c to 779f77e Compare September 24, 2024 13:14
@frouene frouene dismissed sbegaudeau’s stale review September 24, 2024 13:38

Changes taken into account

@frouene frouene merged commit 9cb9436 into eclipse-sirius:master Sep 24, 2024
3 checks passed
@frouene frouene deleted the jgo/enh/add_tree_event branch September 24, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frouene frouene frouene approved these changes

@sbegaudeau sbegaudeau sbegaudeau left review comments

@pcdavid pcdavid Awaiting requested review from pcdavid pcdavid is a code owner

@AxelRICHARD AxelRICHARD Awaiting requested review from AxelRICHARD

Assignees
No one assigned
Labels
pr: to review soon priority: low
Projects
None yet
Milestone
2024.9.0
Development

Successfully merging this pull request may close these issues.

Add a new tree event to handle tree description which are not explorer-related
3 participants
@jerome-obeo @frouene @sbegaudeau